Refactored the get_detailed_services to stop using marshmallow.

Also removed an extra query to services.
The query has been refactored to use an outer join to services on the notifications or notification_history table.
The expectation is that this change will improve the performance of the trial/live-services pages for platform admins.
This commit is contained in:
Rebecca Law
2017-10-26 12:15:52 +01:00
parent 61a68a2d13
commit 1998034b52
4 changed files with 50 additions and 28 deletions

View File

@@ -369,7 +369,7 @@ def dao_fetch_monthly_historical_stats_for_service(service_id, year):
@statsd(namespace='dao')
def dao_fetch_todays_stats_for_all_services(include_from_test_key=True):
def dao_fetch_todays_stats_for_all_services(include_from_test_key=True, only_active=True):
subquery = db.session.query(
Notification.notification_type,
@@ -394,19 +394,24 @@ def dao_fetch_todays_stats_for_all_services(include_from_test_key=True):
Service.name,
Service.restricted,
Service.research_mode,
Service.active,
Service.created_at,
subquery.c.notification_type,
subquery.c.status,
subquery.c.count
).join(
).outerjoin(
subquery,
subquery.c.service_id == Service.id
).order_by(Service.id)
if only_active:
query = query.filter(Service.active)
return query.all()
@statsd(namespace='dao')
def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True):
def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True, only_active=True):
start_date = get_london_midnight_in_utc(start_date)
end_date = get_london_midnight_in_utc(end_date + timedelta(days=1))
table = NotificationHistory
@@ -435,13 +440,17 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro
Service.name,
Service.restricted,
Service.research_mode,
Service.active,
Service.created_at,
subquery.c.notification_type,
subquery.c.status,
subquery.c.count
).join(
).outerjoin(
subquery,
subquery.c.service_id == Service.id
).order_by(Service.id)
if only_active:
query = query.filter(Service.active)
return query.all()

View File

@@ -409,26 +409,33 @@ def get_detailed_service(service_id, today_only=False):
def get_detailed_services(start_date, end_date, only_active=False, include_from_test_key=True):
services_old = {service.id: service for service in dao_fetch_all_services(only_active)}
if start_date == datetime.utcnow().date():
stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key)
stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=include_from_test_key,
only_active=only_active)
else:
stats = fetch_stats_by_date_range_for_all_services(start_date=start_date,
end_date=end_date,
include_from_test_key=include_from_test_key)
services = {service.service_id: service for service in stats}
include_from_test_key=include_from_test_key,
only_active=only_active)
results = []
for service_id, rows in itertools.groupby(stats, lambda x: x.service_id):
services[service_id].statistics = statistics.format_statistics(rows)
# for service_id, rows in itertools.groupby(stats, lambda x: x.service_id):
# services[service_id].statistics = statistics.format_statistics(rows)
# if service has not sent anything, query will not have set statistics correctly
for service in services.values():
if not hasattr(service, 'statistics'):
service.statistics = statistics.create_zeroed_stats_dicts()
return services
rows = list(rows)
if rows[0].count is None:
s = statistics.create_zeroed_stats_dicts()
else:
s = statistics.format_statistics(rows)
results.append({
'id': str(rows[0].service_id),
'name': rows[0].name,
'notification_type': rows[0].notification_type,
'research_mode': rows[0].research_mode,
'restricted': rows[0].restricted,
'active': rows[0].active,
'created_at': rows[0].created_at,
'statistics': s
})
return results
@service_blueprint.route('/<uuid:service_id>/whitelist', methods=['GET'])

View File

@@ -682,7 +682,7 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_services(notify_db
assert stats == sorted(stats, key=lambda x: x.service_id)
def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db):
def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db, notify_db_session):
with freeze_time('2001-01-01T23:59:00'):
just_before_midnight_yesterday = create_notification(notify_db, None, to_field='1', status='delivered')
@@ -712,10 +712,14 @@ def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db, not
stats = dao_fetch_todays_stats_for_all_services()
assert len(stats) == 4
assert (service1.id, service1.name, service1.restricted, service1.research_mode, 'sms', 'created', 2) in stats
assert (service1.id, service1.name, service1.restricted, service1.research_mode, 'sms', 'failed', 1) in stats
assert (service1.id, service1.name, service1.restricted, service1.research_mode, 'email', 'created', 1) in stats
assert (service2.id, service2.name, service2.restricted, service2.research_mode, 'sms', 'created', 1) in stats
assert (service1.id, service1.name, service1.restricted, service1.research_mode, service1.active,
service1.created_at, 'sms', 'created', 2) in stats
assert (service1.id, service1.name, service1.restricted, service1.research_mode, service1.active,
service1.created_at, 'sms', 'failed', 1) in stats
assert (service1.id, service1.name, service1.restricted, service1.research_mode, service1.active,
service1.created_at, 'email', 'created', 1) in stats
assert (service2.id, service2.name, service2.restricted, service2.research_mode, service2.active,
service2.created_at, 'sms', 'created', 1) in stats
def test_dao_fetch_todays_stats_for_all_services_includes_all_keys_by_default(notify_db, notify_db_session):
@@ -754,7 +758,8 @@ def test_fetch_stats_by_date_range_for_all_services(notify_db, notify_db_session
assert len(results) == 1
assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted,
result_one.service.research_mode, 'sms', 'created', 2)
result_one.service.research_mode, result_one.service.active,
result_one.service.created_at, 'sms', 'created', 2)
@freeze_time('2001-01-01T23:59:00')
@@ -794,7 +799,8 @@ def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(n
assert len(results) == 1
assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted,
result_one.service.research_mode, 'sms', 'created', int(expected))
result_one.service.research_mode, result_one.service.active, result_one.service.created_at,
'sms', 'created', int(expected))
@pytest.mark.parametrize("start_delta, end_delta, expected",
@@ -822,7 +828,8 @@ def test_fetch_stats_by_date_range_during_bst_hour_for_all_services_returns_test
assert len(results) == 1
assert results[0] == (result_one.service.id, result_one.service.name, result_one.service.restricted,
result_one.service.research_mode, 'sms', 'created', int(expected))
result_one.service.research_mode, result_one.service.active, result_one.service.created_at,
'sms', 'created', int(expected))
@freeze_time('2001-01-01T23:59:00')

View File

@@ -1753,8 +1753,7 @@ def test_get_detailed_services_only_includes_todays_notifications(notify_db, not
@pytest.mark.parametrize(
'set_time',
['2017-03-28T12:00:00']
#, '2017-01-28T12:00:00', '2017-01-02T12:00:00', '2017-10-31T12:00:00']
['2017-03-28T12:00:00', '2017-01-28T12:00:00', '2017-01-02T12:00:00', '2017-10-31T12:00:00']
)
def test_get_detailed_services_for_date_range(notify_db, notify_db_session, set_time):
from app.service.rest import get_detailed_services